-
Notifications
You must be signed in to change notification settings - Fork 8k
driver: espi: implement espi PM function #94200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
driver: espi: implement espi PM function #94200
Conversation
JasonLin-RealTek
commented
Aug 7, 2025
- add cs pin as espi driver wake up reference
- removed the unnecessary update of the cached date
- removed the unnecessary busy wait in notify funciton
drivers/espi/espi_realtek_rts5912.c
Outdated
#include <zephyr/pm/device.h> | ||
#include <zephyr/pm/policy.h> | ||
#include "reg/reg_system.h" | ||
#define RTS5912_SCCON_REG_BASE ((SYSTEM_Type *)(DT_REG_ADDR(DT_NODELABEL(sccon)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this macro already defined by reg/reg_system.h?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
drivers/espi/espi_realtek_rts5912.c
Outdated
#define ESPI_RTS5912_ESPI_CS_PORT DEVICE_DT_GET(DT_NODELABEL(gpioa)) | ||
#define ESPI_RTS5912_ESPI_CS_PIN 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a property to the realtek,rts5912-espi
node to get the wakeup GPIO information instead of hardcoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use gpios = <&gpioa 4 0>;
in device tree to get CS Pin Information, thanks.
419e686
to
12ad3ef
Compare
drivers/espi/espi_realtek_rts5912.c
Outdated
gpio_flags_t cs_pin_config; | ||
|
||
gpio_pin_get_config(port, pins, &cs_pin_config); | ||
if (cs_pin_config & GPIO_GCR_INTEN_Msk) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GPIO_GCR_INTEN_Msk is an internal bit for the GPIO controller. You should check standard flags defined by gpio.h and gpio_defines.h
if (cs_pin_config & GPIO_GCR_INTEN_Msk) { | |
if (cs_pin_config & GPIO_INT_ENABLE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed.
pinctrl-names: | ||
required: true | ||
|
||
gpios: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name this cs-gpios
. You should also add some help text to describe this property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and add the description in yaml.
dts/arm/realtek/ec/rts5912.dtsi
Outdated
0x400a0100 0x1c /* KBC */ | ||
0x400B1600 0xd0>; /* MBX */ | ||
0x400B1600 0xd0 /* MBX */ | ||
0x40090010 0x04>; /* CS pin */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cs_low reg entry has been replaced by the gpios
property, so this is no longer used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed it now.
0x40090010 0x04>; /* CS pin */
12ad3ef
to
9c30eba
Compare
dts/arm/realtek/ec/rts5912.dtsi
Outdated
"emi7", "oob_tx", "oob_rx", | ||
"oob_chg", "maf_tr", "flash_chg", | ||
"port80", "mbx"; | ||
"port80", "mbx", "cs_low"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the cs_low
interrupt is also unused. Please delete from both interrupt-names
and from interrupts
above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cs_low
is still listed in the interrupt names, and interrupt <4 0>
is still listed in interrupt properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, delete them now.
9c30eba
to
d56a0c9
Compare
d56a0c9
to
da5a6ee
Compare
8c0a1cf
da5a6ee
to
8c0a1cf
Compare
@JasonLin-RealTek hey give this a rebase (on current main), there was a transient CI issue. |
8c0a1cf
to
da21ff3
Compare
da21ff3
to
afd3423
Compare
Hi @fabiobaltieri , fixed, thanks. |
|
||
cs-gpios: | ||
description: | | ||
select the cs pin to support the espi wakeup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a note that this property is required if CONFIG_PM is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a note now, thanks.
1. add cs pin as espi driver wake up reference 2. removed the unnecessary update of the cached date 3. removed the unnecessary busy wait in notify funciton Signed-off-by: Lin Yu-Cheng <[email protected]>
afd3423
to
1dee6d6
Compare
|